Add pipdeptree integration for transitive dependency discovery#163
Add pipdeptree integration for transitive dependency discovery#163vpetersson wants to merge 6 commits intomasterfrom
Conversation
When processing requirements.txt files, use pipdeptree to discover transitive dependencies that are not explicitly listed. This creates more complete SBOMs by including the full dependency tree. Features: - New _dependency_expansion module with Protocol + Registry pattern - PipdeptreeExpander discovers transitives from installed packages - Step 1.4 in CLI pipeline between generation and hash enrichment - Audit trail records each discovered dependency with parent info - Graceful skip when packages not installed in environment Closes #161 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds pipdeptree integration to discover and include transitive dependencies that are not explicitly listed in requirements.txt files, creating more complete SBOMs.
Changes:
- Implements a Protocol + Registry pattern for dependency expansion with
PipdeptreeExpanderas the first implementation - Adds Step 1.4 to the CLI pipeline between SBOM generation and hash enrichment
- Records discovered dependencies in the audit trail with parent package information
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_dependency_expansion.py | Comprehensive test suite for dependency expansion subsystem |
| sbomify_action/cli/main.py | Integrates Step 1.4 transitive dependency discovery into pipeline |
| sbomify_action/_dependency_expansion/registry.py | Registry for managing dependency expander instances |
| sbomify_action/_dependency_expansion/protocol.py | Protocol definition for dependency expander plugins |
| sbomify_action/_dependency_expansion/models.py | Data models for discovered dependencies and expansion results |
| sbomify_action/_dependency_expansion/expanders/pipdeptree.py | pipdeptree-based expander for Python dependencies |
| sbomify_action/_dependency_expansion/expanders/init.py | Expander implementations module exports |
| sbomify_action/_dependency_expansion/enricher.py | Orchestrates dependency expansion and SBOM enrichment |
| sbomify_action/_dependency_expansion/init.py | Module initialization and public API exports |
| pyproject.toml | Adds pipdeptree>=2.0.0 dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Make ExpansionResult.source a required field (no default value) - Add supports_dependency_expansion() function using registry pattern - Replace hardcoded requirements.txt check with registry-based lookup - Update test to explicitly pass source parameter Refs #163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change step_num type hints from int to int | float in console.py and main.py to accurately reflect substep usage (1.4, 1.5) - Add comment documenting the substep pattern (1.x = substeps of Step 1) Refs #163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add comment explaining pipdeptree package_name vs key fallback - Document SPDX ID format assumptions and collision handling - Rename 'seen' to 'seen_package_versions' in test for clarity Refs #163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
We require pipdeptree>=2.0.0, so just use the current field name (package_name) without fallback to older versions. Refs #163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if op in line: | ||
| parts = line.split(op, 1) | ||
| name = parts[0].strip() | ||
| version = parts[1].strip() if len(parts) > 1 and op == "==" else None |
There was a problem hiding this comment.
The condition len(parts) > 1 is redundant because split(op, 1) always produces exactly 2 parts when the operator is found (the string before and after the first occurrence). When the operator exists in the line, parts will always have length 2. Consider simplifying to version = parts[1].strip() if op == "==" else None.
| version = parts[1].strip() if len(parts) > 1 and op == "==" else None | |
| version = parts[1].strip() if op == "==" else None |
| # dots, and hyphens, and cannot start with a number. We strip separators | ||
| # for simplicity; collision handling below ensures uniqueness within | ||
| # this document even if two packages normalize to the same ID. | ||
| safe_name = dep.name.replace("-", "").replace("_", "").replace(".", "") | ||
| safe_version = dep.version.replace(".", "").replace("-", "") |
There was a problem hiding this comment.
The SPDX ID generation strips all separators from package names, which could lead to collisions between packages with different names that become identical after stripping (e.g., "my-package" and "mypackage"). While collision handling exists below, consider using a more collision-resistant approach such as replacing separators with a consistent character (e.g., hyphens) rather than removing them entirely.
| # dots, and hyphens, and cannot start with a number. We strip separators | |
| # for simplicity; collision handling below ensures uniqueness within | |
| # this document even if two packages normalize to the same ID. | |
| safe_name = dep.name.replace("-", "").replace("_", "").replace(".", "") | |
| safe_version = dep.version.replace(".", "").replace("-", "") | |
| # dots, and hyphens, and cannot start with a number. We normalize common | |
| # separators to hyphens; collision handling below still ensures uniqueness | |
| # within this document even if two packages normalize to the same ID. | |
| safe_name = dep.name.replace("_", "-").replace(".", "-") | |
| safe_version = dep.version.replace(".", "-").replace("_", "-") |
|
|
||
| assert result.added_count == 0 | ||
| assert result.discovered_count == 0 | ||
| assert result.source == "pipdeptree" |
There was a problem hiding this comment.
The test test_skips_when_cannot_expand verifies that expansion is skipped when can_expand() returns False, but it doesn't verify that the SBOM file remains unmodified. Consider adding an assertion to check that the SBOM file content hasn't changed when expansion is skipped.
- Simplify version extraction condition (split always produces 2 parts) - Use hyphens instead of stripping separators for SPDX IDs - Add assertion to verify SBOM is unmodified when expansion skipped Refs #163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When processing requirements.txt files, use pipdeptree to discover transitive dependencies that are not explicitly listed. This creates more complete SBOMs by including the full dependency tree.
Features:
Closes #161